Skip to content

feat!: upgrade DataFusion to 55 - #8555

Open
discord9 wants to merge 44 commits into
GreptimeTeam:mainfrom
discord9:omos/datafusion-54-upgrade
Open

feat!: upgrade DataFusion to 55#8555
discord9 wants to merge 44 commits into
GreptimeTeam:mainfrom
discord9:omos/datafusion-54-upgrade

Conversation

@discord9

@discord9 discord9 commented Jul 17, 2026

Copy link
Copy Markdown
Contributor

I hereby agree to the terms of the GreptimeDB CLA.

Refer to a related PR or issue link (optional)

What's changed and what's your intention?

This PR upgrades GreptimeDB from the DataFusion 53.1 dependency line to DataFusion 55.0.0, Arrow 59.2, and Parquet 59.2. The branch now merges main at 38577c8854cb76bd3783e3b57aec310496d82c10 without rewriting its existing history; the review-fix head is 400bc00d647d3f94b912d9f19bb28b443f555041.

The upgrade adapts GreptimeDB planning and execution integrations to DataFusion 55 while retaining the behaviors that are material to GreptimeDB: distributed ordering via EnforceSorting, custom physical plans and expression traversal, recursive MergeScan fallback handling, global Top-K through MergeSort, table-function planning, PostgreSQL compatibility, generated range-query validation, count_hash compatibility, JSON field ordering, and topology-specific SQLness plan expectations.

Dependency versions and pins

Dependency Revision / version Purpose
DataFusion exact 55.0.0; fork b66dca3260c8e04f314c8036cdfbb08f1a564d95 DataFusion 55 plus the retained GreptimeDB correctness and repair patches below
Arrow / Parquet exact 59.2.0 DataFusion 55 dependency line
datafusion-orc released 0.10.0 DataFusion 55 ORC integration
orc-rust released 0.9.0 Arrow 59-compatible ORC reader
datafusion-pg-catalog / arrow-pg 3c77e6c32b8d DataFusion 55 PostgreSQL catalog and Arrow integration
sqlparser 0.62.0 at 9e9019bb Preserves GreptimeDB range-query extensions

All 15 DataFusion entries in [patch.crates-io] use the same immutable fork revision, b66dca3260c8e04f314c8036cdfbb08f1a564d95; the corresponding 35 Cargo.lock source blocks use that same revision. The released ORC packages replace the temporary ORC git patches, and datafusion-pg-catalog and arrow-pg remain pinned together at their DataFusion 55-compatible commit.

Thin-fork history

The fork starts at official DataFusion 55.0.0 commit d555234201. The six original retained commits remain for these purposes:

Topic Thin-fork commit Purpose
Global multi-partition fetch / LIMIT 96625f97aa Preserve global fetch semantics for multi-partition input; ports apache/datafusion#23800
Duplicate Substrait root names d1dd7b9ad6 Preserve duplicate root names with deterministic internal qualifiers
Safe cast predicate preimages da6529d542 Rewrite safe-cast predicates through exact or range preimages
Ordered timestamp widening e47063d1e4 Preserve correct ordered timestamp-widening predicate rewrites
Chained logical projections 6922ddce3 Restore one-layer-per-pass projection merging and prevent incorrect nested-projection results
Custom float hash representations dec6af133d Preserve floating-point hash representations required by count_hash state compatibility

The scalar-planner hook restoration aab310aac36 is retained for JSON2. Four additional fork commits are now pushed: 72872aedb (lint-only timestamp conversion cleanup), a17899ec6 (recursive-CTE schema metadata), 85fc18b51 (Date32 predicate widening), and 7ac19906c (cross-timezone regression correction).

Additional retained behaviors after the old-fork audit

The final fork contains 20 commits beyond official DataFusion 55. The old DataFusion 53.1 fork was checked commit-by-commit (23 non-merge commits), including behaviors already supplied upstream rather than only matching patch titles. Five follow-up commits restore six missing repair topics:

Commit Why needed / what changed
05fe011bd7 Ports apache/datafusion#25065. Distribution enforcement must not remove fetched global boundaries or downgrade fetched ordered merges. It now preserves those nodes, including fetch zero, and protects ordered descendants before recursion. This is distinct from the existing LimitPushdown patch.
68fac3de71 Required ordering was read too late for an inner LocalLimit or a removed no-op GlobalLimit. Accumulate it before early rewrites so global selection retains a sort-preserving merge.
84e55f0c6b Restores the old fork's execution-error classification when every OneOf function signature fails; ordinary invalid arguments must not be described as a DataFusion internal bug.
e2b6246f37 Restores Dictionary literal serialization: encode the underlying value and preserve the dictionary expression type through a cast. Existing dictionary schema support alone does not provide this.
579d2c9b9a Restores dictionary-specialized grouping columns using DF55's existing factory, with cross-batch value deduplication, NULL handling, collision checks and typed output. Also clears drained group indexes on full emission so reuse is safe.

Already-upstream retention includes finer timestamp coercion and safe overflow handling, preserved-side join dynamic filters, aggregate filter ordering, SPM statistics/cardinality, dictionary-regex NULL handling, and join-estimation overflow protection. Aligned timestamp widening equality/IN is restored under the explicitly accepted extreme-range overflow policy; unsafe dictionary casts remain restricted. Ordered timestamp widening retains its inherited overflow policy, not full-domain CAST/TRY_CAST equivalence.

DB-local sorting, opaque MergeSort global Top-K handling, and post-optimizer Filter-fetch repairs remain separate from the fork patches. count_hash preservation requires both the DB's fixed-seed custom-hasher API and the fork's raw floating-point representation; this is not a claim that all DataFusion hashing APIs are interchangeable.

Targeted repairs

JSON2 scalar-function planning

  • Why: DataFusion 55 no longer called the scalar ExprPlanner hook GreptimeDB uses to supply JSON2 extraction types. Removing the obsolete GreptimeDB method would compile but makes expressions such as abs(j.metrics.value) fail during planning before analyzer rules can repair them.
  • What: The retained scalar-planner patch restores the existing RawScalarExpr / plan_scalar extension point after scalar argument resolution. Modified Original arguments continue through later planners and normal function construction. GreptimeDB keeps its production JSON2 type injection; this does not introduce a test-only path or a new rewrite framework.

PromQL temporality join-key encoding

  • Why: Stored primary-key labels use Arrow dictionary encoding. When a PromQL operand lacks otlp_aggregation_temporality, the planner supplies a NULL marker so that it matches cumulative (NULL) rows but not delta rows. Remote planning can decode this synthetic literal to plain UTF8 while the stored marker remains dictionary-encoded. DataFusion 55's multi-key hash-join comparator rejects those mixed array types, breaking arithmetic and AND in the existing delta_temporality SQLness case.
  • What: Commit ffba905e4cd decodes only the present dictionary marker to its string value type and supplies the missing marker as a typed NULL of that same type. Qualified column names, NULL matching, and the left operand's visible columns and values for AND/UNLESS are preserved; padding is not exposed for a left operand lacking the temporality marker. This is a local PromQL alignment fix, not a global storage conversion or another fork patch. Tests cover both operand orders, both dictionary NULL representations, exact arithmetic/set results, and a nested consumer. The original stored-primary-key SQLness expectations remain unchanged.

Recursive CTE schema metadata

  • Why: The stream-backed-scan reopening fix in GreptimeDB fix: re-scan stream-backed tables in recursive CTEs #9039 addresses execution-time reuse across recursive iterations. Its query over information_schema.tables also exposes a distinct DataFusion 55 physical-planning failure: Cannot project plan to expected schema: schema metadata differ. A literal seed and table-backed recursive branch may have different top-level metadata even though the declared recursive output takes the seed metadata; the query must continue to return depths 0, 1, 2.
  • What: Commit a17899ec6 makes the existing project_plan_to_schema helper construct ProjectionExec with the declared output schema metadata rather than reject only a top-level metadata difference. The exact-schema fast path remains. Column-count, data-type, nullability-narrowing, and field-metadata mismatches remain rejected. It neither strips scan metadata nor changes fix: re-scan stream-backed tables in recursive CTEs #9039's stream-reopening behavior.

Date32 predicate optimization restoration

  • Why: The safe-cast preimage allowlist omitted lossless, order-preserving Date32 → Date64, so existing simplify_expr.slt EXPLAIN cases could not eliminate those casts. Every i32 day value scaled by 86,400,000 fits in i64.
  • What: Commit 85fc18b51 allows only Date32 → Date64. Inverse-literal whole-day alignment and Date32-domain checks remain, and Date64 → Date32 remains rejected. Coverage includes boundaries, ordered comparisons, nonaligned and out-of-range literals, and reverse-direction rejection.

Cross-timezone equality regression

  • Why: A prior SLT positive case treated Brussels 2024-02-01 00:00:01 +01 and Los Angeles 2024-01-31 16:00:01 -08 as equal, although they differ by one hour. Because the view explicitly casts the input to timestamp before assigning Brussels, its matching Los Angeles wall clock is 15:00:01.
  • What: Commit 7ac19906c changes the positive literal to 15:00, retains the expected positive row, and preserves 16:00 as a no-match regression. It changes neither timezone implementation nor the positive result semantics.

The lint-only commit 72872aedb removes four redundant i128::from(timestamp_unit_scale(...)) conversions: the helper already returns i128, so strict clippy rejects these identity conversions. Timestamp arithmetic and guards are unchanged.

EnforceSorting remains conservatively preserved to retain the existing GreptimeDB sorting pass and distributed plan shaping during this upgrade. This does not establish that an extra pass is intrinsically required beyond DataFusion’s default EnsureRequirements.

User-visible changes and compatibility limits

This upgrade is not a blanket SQL/API compatibility guarantee:

  • Integer-input median, approx_median, and approx_percentile_cont produce floating-point results. Exact median interpolates even-sized inputs (for example, 4 becomes 4.5 in the existing regression); approximate quantile values also change. Consumers with fixed output schemas, including views and Flow sinks, need compatibility assessment; those schema migrations have not been exhaustively tested here.
  • REPLACE(s, '', replacement) now returns s unchanged instead of interleaving the replacement string.
  • Struct unnest output column names drop the __unnest_placeholder(...) wrapper. Clients depending on generated names should use explicit aliases.
  • Approximate cardinality estimates can change: the existing 2,000-value approx_distinct case changes from 2000 to 1991. This does not promise stable estimates across versions.
  • Empty pretty-printed query results retain column headers rather than rendering as ++; this is a display change, not added result rows.
  • PostgreSQL result metadata honors OID-alias annotations (OID, REGCLASS, REGTYPE, and related types) instead of reporting only their former INT4/VARCHAR representations.
  • Global serde_json/preserve_order is no longer enabled in the verified production graph. Both sources were removed: Substrait named-struct ordinals use a local IndexMap, and physical EXPLAIN JSON uses local ordered serialization. General DB JSON object ordering remains consistent with pre-upgrade main; SST metadata expectations were regenerated accordingly. The focused raw-byte CAS/checksum audit found no issue in the inspected paths, but is not a universal byte-stability claim.

Timestamp precision-widening equality and IN preimages are restored by fork commit 605effa378, reusing the existing literal conversion and round-trip alignment checks. Same-timezone widening is eligible; narrowing, nonaligned literals and unsafe dictionary casts remain guarded. The explicitly accepted policy favors normal-range time pruning even when rewriting changes CAST/TRY_CAST overflow behavior at extreme timestamps; no full-domain equivalence is claimed. Date32-to-Date64 remains eligible as well.

Standalone and distributed SQLness both verify ts = TimestampMillisecond(5000) reaches scan filters without CAST; aligned IN becomes source-column equalities. Exact/nonexact normal-range results and the accepted extreme-range policy are covered. Direct arrow_try_cast returns NULL on the tested positive overflow. The pre-existing SQL TRY_CAST(... AS TIMESTAMP(9)) type-alias-lowering limitation remains outside this upgrade's scope.

Substrait 0.63 is patched via GreptimeTeam/substrait-rs revision 91ec978b0649417ad3da8390e7a515baec723b1b. Its named-struct field order remains explicit and local; no protobuf/spec upgrade or DB-wide ordered-JSON abstraction is introduced. The raw Type::Object payload is now IndexMap (a Rust source API adjustment); legacy conversion helpers are retained. The fork also supports Cargo tagless checkout of the exact pinned spec release.

Review follow-ups

  • Restore primitive Signature::uniform contracts for clamp/min/max, modulo, rate and anomaly functions. Decimal inputs follow the legacy Float64 coercion path instead of reaching unsupported native-decimal execution. Actual Decimal128 coercion/execution regressions and standalone/distributed SQL column queries cover clamp and modulo. This does not add native Decimal arithmetic support.
  • Restore scalar count_hash typed-NULL behavior, including its historical zero hash contribution. Preserve fixed seeds, raw floating-point hashing and state encoding; a persisted [0] state regression checks state/result agreement with fresh typed-NULL input. The pre-existing scalar/group NULL inconsistency is deliberately not corrected in this upgrade.
  • Remove broad SortExec/topology masking. Separate standalone/distributed TQL expectations now expose their real sort keys and distinct plan shapes. PostgreSQL catalog checks explicitly assert attnum/column-name/type associations in ordinal order.
  • Share only identical geo integer-signature lists in the existing helper module, and simplify reviewed type imports. Numeric and integer-only lists remain separate.

Validation and PR state

Historical timestamp-restoration validation (400bc00d647): fork 605effa378 passes 9 expr-common, 12 logical optimizer and 58 physical simplifier tests plus strict three-crate all-target/all-feature Clippy. Greptime vector-index build and formatting pass; query test result: Summary [ 4.896s] 739 tests run: 739 passed, 2 skipped. Both deployment cast-preimage SQLness reruns pass with explicit bare-millisecond EQ/IN scan predicates. At head 400bc00d647, Rust CI passed and Integration CI run 34457426139 attempt 2 passed, including actually executed recent-release compatibility. Logical-pruning fuzz initially failed after a candidate datanode health-probe timeout/restart, then passed on retry; the underlying stall is not established. Cargo.lock Diff Check failed while posting its size warning because the integration lacked comment permissions, not because dependency resolution failed.

Earlier review-fix head dcaedac52cd: cargo nextest run --locked -p common-function passes 343 tests, including Decimal coercion/execution and historical count_hash state restoration. Strict all-target Clippy for common-function and catalog, the vector-index-enabled greptime build, and workspace Rust formatting pass. Actual Decimal-column SQL queries produce the expected clamp/min/max/modulo results in standalone and distributed deployments; the generated outputs are identical and were rerun successfully.

Review SQLness validation passes in both deployments: explicit sorting/column-ordinal assertions, SST metadata output, and timestamp safe/overflow boundary cases. Relocated TQL fixtures were additionally rerun at their final paths. That earlier forced-feature snapshot check is historical; current SST output is verified without global preservation. Expected files were generated by SQLness, not hand-edited.

Earlier merge validation (a5aaebe5cc4 / 4321681d53f): vector-index cmd check, query/table/promql tests (1038 passed, 2 skipped), and both binary builds pass. Nineteen merge-sensitive SQLness cases have passing results after regenerating the two new Arrow59 empty-table renderings. CI on 4321681d53f passed all executed build, Rust, Clippy, SQLness, SDK and Fuzz jobs; recent-releases compatibility was skipped on synchronize. The Cargo.lock advisory job failed while attempting to post its large-diff warning (Resource not accessible by integration), not while resolving dependencies.

Earlier retention-pin validation (e749334f0d4): query tests 693 passed, 2 skipped, and 38 targeted SQLness executions passed. Recent-releases compatibility actually executed and passed in Integration run 34350572598, attempt 2. This is historical evidence, not a compatibility pass for the current review-fix head.

Fork 579d2c9b9a: combined validation passes 569 physical-optimizer tests, 82 group-value tests, 57 Substrait unit tests, 202 Substrait integration tests (6 ignored), 3 doctests, and 31 coercion tests. Affected crates pass strict all-target/all-feature Clippy. Dictionary-focused normal and forced-collision runs each pass 79 tests. Native before/after regressions confirm LIMIT 1 and LIMIT 0 survive repeated optimization and ordered TopK retains [10,20] rather than four rows.

Historical head 8b12b1b67ae: Rust CI 34475382126 and Integration CI 34480886694 passed, including actually executed recent-releases compatibility. The Cargo.lock warning-comment check remains red due to GitHub API comment permissions, not dependency resolution or test failure. This PR remains Ready for review and has not been merged.

PR Checklist

  • I have written the necessary rustdoc comments.
  • I have added the necessary unit tests and integration tests.
  • This PR requires documentation updates.
  • API changes are backward compatible. (User-visible SQL result types, labels, and PostgreSQL wire metadata change; see above.)
  • Schema or data changes are backward compatible. (Current merge compatibility CI pending; prior-head pass is historical.)
  • This PR needs to be backported to release branches.

SQL23 correctness follow-up

The fork now includes ca00e5c7f, backporting merged Apache DataFusion #24817 (b3cb365dd6e5e0441f3f58817fee55dae1814fdb). Aggregate dynamic filtering is disabled for the entire aggregate if any MIN/MAX argument is unsupported; filtering only for the supported subset can discard rows required by expressions such as MAX(ABS(v)). All-direct-column MIN/MAX OR filtering remains enabled.

Validation of this fork backport: the complete push_down_filter_regression.slt passes, including deterministic mixed-aggregate numeric-result and no-filter assertions; formatting and full-workspace cargo clippy --all-targets --all-features -- -D warnings pass. Independent review approved the upstream backport. These are fork-level checks. DB validation is recorded below: SQL23 HTTP verification at 5db4efe77f4, and successful integration/compatibility CI at current head 8b12b1b67ae.

The separate review of 384 upstream commits from DF55 to frozen upstream main 2db0ec3bbedb72e7fc68938feef4b4b70efa7f93 is still selecting applicable fixes. The three expression fixes listed below are now included; other selected candidates remain pending and are not part of this update.

NULL and predicate simplification follow-up

Fork commit ad5c4be23 backports three merged upstream correctness fixes:

  • #24248: preserve NULL semantics when cancelling repeated operands in complex bitwise XOR expressions.
  • #24380: preserve NULL rather than returning false for projected col ~ '.*'.
  • #24763: normalize literal-left comparisons before merging predicates, avoiding incorrect contradictions.

Verification: 759 optimizer unit tests pass; complete scalar.slt, simplify_expr.slt, and simplify_predicates.slt pass; formatting and full-workspace all-target/all-feature Clippy with -D warnings pass. Independent review confirmed upstream fidelity. Stale timestamp SLT expectations were aligned with the already accepted widening policy; production timestamp behavior was not changed. These are fork-level results. At Greptime head 8b12b1b67ae, Rust CI run 34475382126 and Integration CI run 34480886694 passed, including actually executed recent-release compatibility (not skipped), SQLness, fuzz/chaos, SDK and export/import checks. The remaining Cargo.lock Diff Check failure is its warning-comment API permission error (Resource not accessible by integration), not dependency resolution or a test failure.

Additional SQL23 DB verification at preceding head 5db4efe77f4: vector-index build passed, query tests 739 passed / 2 skipped, and the real HTTP reproducer returned the correct mixed aggregate [1000000, 1700000019999] in all 20 repetitions, with no incomplete aggregate dynamic filter in EXPLAIN ANALYZE. This is not a claim of DB verification for the newer expression-fix pin.

The selected Parquet statistics, DISTINCT/fetch/OFFSET, legacy aggregate spill, and window-argument fixes have not yet been backported and are not included.

Current merge and JSON-order isolation — bc7ecdce88f

Merged main 9dabe8ccd232739614b21119bc63116567538653 into this branch (no rebase). Kept main’s proto/jemallocator updates and SQL23 regression tests, retained DF55, and pinned DF b66dca326 plus Substrait 91ec978. No additional deferred upstream fixes are included.

Validation: published Git pins resolve and build; locked cmd + vector_index normal/build feature graph contains no serde_json/preserve_order; DB Substrait/datasource/query tests 787 passed, 2 skipped. Equivalent local-source integration ran 44 actual SQLness executions across standalone/distributed JSON, SST, SQL23 and applicable copy cases, then reran green after generated SST key-order expectations returned to the main baseline. Physical pgjson tests pass in both feature states and full DataFusion strict Clippy passes; Substrait has 59 unit + 3 doc tests passing.

New-head CI and recent-release compatibility are pending. Previous-head CI results above are historical, not validation of this merge. The unresolved JSON review thread has not been automatically resolved or replied to.

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request upgrades the DataFusion dependency from version 53.1.0 to 54.0.0, which involves extensive refactoring to adapt to API changes such as the removal of explicit as_any() downcasting, updates to the Cast expression structure, and changes to PruningStatistics and partition_statistics signatures. It also introduces a frozen hash_v1 module to preserve compatibility with legacy count_hash states. The review identified critical compilation errors: a missing import for as_largestring_array in the new hash_v1.rs file, and incorrect iterations over the return value of hash_utils::create_hashes in union_distinct_on.rs since it now returns Result<()> instead of the hashes.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment thread src/common/function/src/aggrs/count_hash/hash_v1.rs Outdated
Comment thread src/promql/src/extension_plan/union_distinct_on.rs Outdated
Comment thread src/promql/src/extension_plan/union_distinct_on.rs Outdated
@github-actions github-actions Bot added docs-not-required This change does not impact docs. breaking-change This pull request contains breaking changes. labels Jul 17, 2026
@sunng87

sunng87 commented Jul 17, 2026

Copy link
Copy Markdown
Member

orc and pg-catalog are our managed libraries, make sure it merges to upstream before merging this.

@sunng87

sunng87 commented Jul 18, 2026

Copy link
Copy Markdown
Member

I'm working on the pg-catalog upgrade for datafusion 54. will commit to this branch.

@discord9

Copy link
Copy Markdown
Contributor Author

Thanks — I see 193726d60a updates pg-catalog to 0.18 and adds the related planner/session compatibility changes. The PR DCO check is currently failing because this commit has no Signed-off-by trailer. Could you please add the sign-off using the DCO app’s accepted flow? I’ll retain and validate this commit while reconciling the latest main; I won’t overwrite the branch update.

@sunng87

sunng87 commented Jul 20, 2026

Copy link
Copy Markdown
Member

DCO is a non-blocking check, just set it passed.

@discord9

Copy link
Copy Markdown
Contributor Author

Thanks, DCO is green now. One remaining managed-dependency setup: GreptimeTeam/datafusion still has main on 53.1, and my direct attempt to create a 54 branch returned HTTP 403 (push=false). Could you create an organization target branch such as greptimedb-54.0.0 from official DataFusion 54.0.0@45d943dfb8699dc9cb9ef2320e955b73e3e6c03b? The validated thin-fork head is discord9/datafusion:thin-fork/2026-07-16-datafusion-54@027358559b74632d3231eea46a34fcc7f7c99fe9; once the target exists I can open the PR and move #8555 to the organization-controlled SHA.

@sunng87

sunng87 commented Jul 20, 2026

Copy link
Copy Markdown
Member

datafusion-orc is now on 0.9

@discord9

Copy link
Copy Markdown
Contributor Author

The pg-catalog 0.18 integration uncovered one remaining upstream compatibility gap during real PostgreSQL-protocol SQLness validation: OID aliases are represented as Int32 + pg.oid_alias metadata, but reverse casts and wire mapping currently expose numeric values (25::regtype::text -> 25, typinput -> 46) instead of PostgreSQL names/types. I filed datafusion-postgres#384 with library-level reproductions. I am fixing the Greptime-owned wire metadata mapping, pg_attribute.attnum ordering, and unstable OID assertions locally; I will not accept the numeric snapshot as the baseline. The symbolic alias-to-text fix belongs upstream and remains a Draft gate.

@discord9

Copy link
Copy Markdown
Contributor Author

Updated locally to the managed datafusion-orc 0.9.0 crates.io release. The temporary personal git source is removed; the locked common-datasource and mito2 checks pass. This will be included in the pending latest-main merge commit.

@discord9

Copy link
Copy Markdown
Contributor Author

Could you confirm the intended ownership for the DataFusion thin-fork pin itself? The existing dependency used GreptimeTeam/datafusion, but the validated DF54 head is currently discord9/datafusion@027358559b...; I cannot create the requested GreptimeTeam 54 target branch (push=false, direct push returns 403). Is moving this SHA into GreptimeTeam/datafusion a hard pre-merge requirement, or is the immutable personal-fork SHA acceptable for this upgrade? I am treating it as a conservative dependency-ownership gate until clarified.

@discord9

Copy link
Copy Markdown
Contributor Author

The managed DataFusion 54 path is now available. I created GreptimeTeam/datafusion:greptimedb-54.0.0 directly from the exact official 54.0.0@45d943dfb... release commit and opened GreptimeTeam/datafusion#20 from the validated thin-fork head 027358559b.... PR #20 is CLEAN and review has been requested from @sunng87. After it merges, #8555 can replace all personal DataFusion URLs with the organization-controlled immutable SHA.

@github-actions github-actions Bot added size/M and removed size/XXL labels Jul 20, 2026
@sunng87 sunng87 linked an issue Jul 20, 2026 that may be closed by this pull request
@github-actions github-actions Bot added size/L and removed size/M labels Jul 20, 2026
@sunng87

sunng87 commented Jul 22, 2026

Copy link
Copy Markdown
Member

datafusion 54.1 is out.
I also have a new release for datafusion-pg-catalog 0.18.2 with some bugfix. Consider sending the quote patch to upstream.

Comment thread src/common/function/src/scalars/geo/h3.rs Outdated
@discord9
discord9 force-pushed the omos/datafusion-54-upgrade branch from fd09114 to cf9188e Compare August 3, 2026 10:28
discord9 added a commit to discord9/greptimedb that referenced this pull request Aug 3, 2026
Address review feedback on PR GreptimeTeam#8555: extract duplicated coordinate x
resolution signature construction, coordinate/resolution argument
parsing, and per-row resolution conversion into shared helpers
(signature_of_coordinates_and_resolution, extract_coordinate_resolution_args,
resolution_at). Drop redundant alias statics (RESOLUTION_TYPES,
DISTANCE_TYPES, POSITION_TYPES) in favor of INTEGER_TYPES. Behavior,
signatures, error messages, and visibility are unchanged.

Signed-off-by: discord9 <discord9@163.com>
@discord9

discord9 commented Aug 3, 2026

Copy link
Copy Markdown
Contributor Author

Addressed: eaf98f1a53 extracts the duplicated coordinate×resolution signature construction, coordinate/resolution argument parsing, and per-row resolution conversion into shared helpers (signature_of_coordinates_and_resolution, extract_coordinate_resolution_args, resolution_at) and drops the redundant RESOLUTION_TYPES/DISTANCE_TYPES/POSITION_TYPES alias statics in src/common/function/src/scalars/geo/h3.rs. Behavior, signature order, error messages, and visibility are unchanged; common-function h3 tests pass (1/1) and clippy/fmt are clean.

@discord9
discord9 marked this pull request as ready for review August 5, 2026 04:37
Signed-off-by: discord9 <55937128+discord9@users.noreply.github.com>
Signed-off-by: discord9 <55937128+discord9@users.noreply.github.com>
Signed-off-by: discord9 <55937128+discord9@users.noreply.github.com>
Signed-off-by: discord9 <55937128+discord9@users.noreply.github.com>
Signed-off-by: discord9 <55937128+discord9@users.noreply.github.com>
Signed-off-by: discord9 <55937128+discord9@users.noreply.github.com>
Merge main at 38577c8 without rewriting the existing PR history. Retain last-row scan hints and dynamic filtering while adapting new main downcasts to DataFusion 55.

Signed-off-by: discord9 <55937128+discord9@users.noreply.github.com>
Signed-off-by: discord9 <55937128+discord9@users.noreply.github.com>
Comment thread src/catalog/src/information_extension.rs Outdated
Comment thread src/common/function/src/aggrs/aggr_wrapper/tests.rs Outdated
Comment thread src/common/function/src/scalars/geo/geohash.rs Outdated
@killme2008

Copy link
Copy Markdown
Member

User-visible behavior changes inherited from the upgrade

Most of these are not introduced by this PR's own code, but they ship with it and are visible to users. They should be listed in the release notes, and the PR checklist needs updating — API changes are backward compatible is currently checked.

Change Source Evidence
median() / approx_median() / approx_percentile_cont() return Float64 for integer input, and even-count medians now interpolate (median(r)::VARCHAR returns 4.5, was 4) DataFusion 55 — functions-aggregate/src/median.rs: "Integer inputs are coerced to Float64 so the average of the two middle values is exact" aggregate/median.result, aggregate/approx_median.result, aggregate/approx_percentile_cont.result, range/calculate.result
REPLACE(s, '', x) returns s unchanged instead of interleaving x between every character (now matches PostgreSQL/MySQL) DataFusion 55 function/string/replace.result
unnest() output columns lose the __unnest_placeholder(...) wrapper: geo_path(lat,lon,ts).lat instead of __unnest_placeholder(geo_path(lat,lon,ts)).lat DataFusion 55 function/geo.result, select/unnest.result
approx_distinct() returns different estimates for the same input (2000 -> 1991) DataFusion 55 aggregate/approx_distinct.result
Empty result sets render with a header row instead of ++ arrow 59 pretty-print 159 occurrences across 66 .result files
PostgreSQL wire types: OID-alias columns are now reported as OID/REGCLASS/REGTYPE/... instead of INT4/VARCHAR this PR (servers/src/postgres/types.rs) postgres/types.rs

The median change is the one with real blast radius: it changes the column type of existing views and flow sink tables built on median(), not just the printed value.

@killme2008 killme2008 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A few things from reading the diff. Breaking changes are in a separate comment above.

One more that has no source line: the description says the current head is e749334f0d4, but the head of this PR is 3b8c0a85b5. None of the validation results in the description refer to the commit being reviewed.

Comment thread tests/cases/standalone/common/filter/cast_preimage.result Outdated
Comment thread tests/cases/standalone/common/tql/general_table.sql Outdated
Comment thread tests/cases/standalone/common/system/pg_catalog.sql Outdated
| data/greptime/public/<TABLE_ID>/ | data/greptime/public/<TABLE_ID>/<REGION_ID>_<REGION_NUMBER>/index/<UUID>.puffin |<NUM>|<NUM>|<NUM>|<NUM>|<NUM>| <UUID> |<NUM>| bloom_filter | column |<NUM>| {"column":1} |<NUM>| {"bloom":{"rows_per_segment":10240,"segment_count":1,"row_count":1,"bloom_filter_size":64}} |<NUM>|
| data/greptime/public/<TABLE_ID>/ | data/greptime/public/<TABLE_ID>/<REGION_ID>_<REGION_NUMBER>/index/<UUID>.puffin |<NUM>|<NUM>|<NUM>|<NUM>|<NUM>| <UUID> |<NUM>| bloom_filter | column |<NUM>| {"column":1} |<NUM>| {"bloom":{"rows_per_segment":10240,"segment_count":1,"row_count":1,"bloom_filter_size":64}} |<NUM>|
| data/greptime/public/<TABLE_ID>/ | data/greptime/public/<TABLE_ID>/<REGION_ID>_<REGION_NUMBER>/index/<UUID>.puffin |<NUM>|<NUM>|<NUM>|<NUM>|<NUM>| <UUID> |<NUM>| bloom_filter | column |<NUM>| {"column":1} |<NUM>| {"bloom":{"rows_per_segment":10240,"segment_count":1,"row_count":1,"bloom_filter_size":64}} |<NUM>|
| data/greptime/public/<TABLE_ID>/ | data/greptime/public/<TABLE_ID>/<REGION_ID>_<REGION_NUMBER>/index/<UUID>.puffin |<NUM>|<NUM>|<NUM>|<NUM>|<NUM>| <UUID> |<NUM>| inverted | column |<NUM>| {"column":0} |<NUM>| {"inverted":{"bitmap_type":"Roaring","base_offset":0,"inverted_index_size":81,"relative_fst_offset":26,"fst_size":55,"relative_null_bitmap_offset":0,"null_bitmap_size":8,"segment_row_count":1024,"total_row_count":1}} |<NUM>|

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The meta_json keys went from sorted to source order here, and puffin_index.rs isn't touched in this PR.

The cause is substrait 0.62.2 -> 0.63.0. 0.63.0 declares serde_json = { version = "1.0.145", features = ["preserve_order"] }, and feature unification turns that on for the whole workspace, so serde_json::Map becomes an IndexMap everywhere. Cargo.lock confirms it: serde_json on main has no indexmap dependency, on this branch it does.

So this affects every JSON we build or round-trip through serde_json::Value, not just this column. JSON column values are fine, they go through jsonb::parse_value. Please call it out in the description, and check that nothing compares or hashes serialized JSON bytes.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated in bc7ecdc. We decided not to accept global serde_json ordering changes as part of this upgrade. The production feature graph revealed two enablers, not just Substrait: substrait 0.63 and datafusion-physical-plan.

Both are now scoped locally:

  • GreptimeTeam/substrait-rs@91ec978 preserves named-struct field order/ordinals with an explicit IndexMap, without enabling serde_json/preserve_order. The existing spec/protobuf version is unchanged; legacy raw-type conversion helpers are retained.
  • DataFusion fork b66dca326 preserves physical EXPLAIN JSON field and metric ordering through renderer-local serialization, including duplicate metric overwrite behavior, without the global feature.

With the published Git pins, the locked cmd + vector_index normal/build dependency graph has no serde_json/preserve_order. General DB JSON ordering returns to the pre-upgrade main behavior; SQLness regenerated the SST meta_json expectation accordingly. Published-pin build and 787 Substrait/datasource/query tests passed (2 skipped); equivalent local-source integration passed 44 SQLness executions. Physical pgjson tests pass with and without preserve_order. New-head CI/compatibility is still pending.

We also checked the targeted byte-sensitive paths: table-info/table-route/table-repartition CAS compares captured original KV bytes, and manifest checksums cover original payload bytes. That is a focused audit, not a claim that every JSON consumer was exhaustively audited. The PR description now documents the local-ordering decision and validation. Leaving this thread open for your review.

Comment thread src/common/function/src/scalars/math/clamp.rs Outdated
Signed-off-by: discord9 <55937128+discord9@users.noreply.github.com>
Signed-off-by: discord9 <55937128+discord9@users.noreply.github.com>
Signed-off-by: discord9 <55937128+discord9@users.noreply.github.com>
Signed-off-by: discord9 <55937128+discord9@users.noreply.github.com>
Signed-off-by: discord9 <55937128+discord9@users.noreply.github.com>
Signed-off-by: discord9 <55937128+discord9@users.noreply.github.com>
Signed-off-by: discord9 <55937128+discord9@users.noreply.github.com>
Signed-off-by: discord9 <55937128+discord9@users.noreply.github.com>
Signed-off-by: discord9 <55937128+discord9@users.noreply.github.com>
Signed-off-by: discord9 <55937128+discord9@users.noreply.github.com>
Signed-off-by: discord9 <55937128+discord9@users.noreply.github.com>
Signed-off-by: discord9 <55937128+discord9@users.noreply.github.com>
Signed-off-by: discord9 <55937128+discord9@users.noreply.github.com>
Signed-off-by: discord9 <55937128+discord9@users.noreply.github.com>
Signed-off-by: discord9 <55937128+discord9@users.noreply.github.com>
Signed-off-by: discord9 <55937128+discord9@users.noreply.github.com>
…pr8555-ci-rebase

Signed-off-by: discord9 <55937128+discord9@users.noreply.github.com>
Signed-off-by: discord9 <55937128+discord9@users.noreply.github.com>
Signed-off-by: discord9 <55937128+discord9@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

breaking-change This pull request contains breaking changes. ci docs-not-required This change does not impact docs. size/XL

Projects

None yet

Development

Successfully merging this pull request may close these issues.

PostgreSQL compat: SELECT array[null] returns incorrect value

5 participants